Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of new SparsityOp #790

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Conversation

Garic152
Copy link
Contributor

This PR introduces a new SparsityOp that allows users to print the compile-time sparsity estimate of a matrix to the screen. The function can be called using sparsity(Matrix), which will be replaced with the sparsity value through canonicalization if the sparsity is known. If the sparsity is unknown, it will return -1.

This PR does not yet include tests. The SparsityOp functionality will be tested as part of #766.

@corepointer corepointer added the feature missing/requested features label Aug 1, 2024
Copy link
Collaborator

@pdamme pdamme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution, @Garic152. The sparsity built-in/op will be useful for testing sparsity estimation for now (maybe we find a better solution in the future, but I think this approach is quite straightforward). Your code looks very good to me. I only applied some minor changes (see my commit). This PR is ready to be merged.

@pdamme
Copy link
Collaborator

pdamme commented Aug 5, 2024

All tests pass successfully on my machine and did pass in the CI before my mostly syntactical changes. At the moment, the CI fails due to problems in setting up the container. Thus, I think it's safe to merge this PR.

@pdamme pdamme merged commit 0542291 into daphne-eu:main Aug 5, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature missing/requested features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants